Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support variables in slice filter shorthand #881

Merged
merged 3 commits into from
Nov 16, 2023
Merged

Support variables in slice filter shorthand #881

merged 3 commits into from
Nov 16, 2023

Conversation

antoineveldhoven
Copy link
Contributor

Resolves #769

@RobLoach RobLoach changed the title Support variables in slice filter shorthand. Support variables in slice filter shorthand Nov 6, 2023
RobLoach
RobLoach approved these changes Nov 6, 2023
View reviewed changes
Copy link
Collaborator

@RobLoach RobLoach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh this is nice. Thanks a lot. Looks good to me. Anything else we should address before merging?

@antoineveldhoven
Copy link
Contributor Author

Oh this is nice. Thanks a lot. Looks good to me. Anything else we should address before merging?

Thanks for the review, this commit should merge perfectly as-is.
Could you or @willrowe take a look at #874, we really need this to me merged for our project(s).

willrowe
willrowe requested changes Nov 7, 2023
View reviewed changes
src/twig.expression.js Show resolved Hide resolved
src/twig.expression.js Show resolved Hide resolved
test/test.expressions.js Show resolved Hide resolved
test/test.filters.js Show resolved Hide resolved
@willrowe willrowe added this to the v1.17.0 milestone Nov 7, 2023
@antoineveldhoven
Copy link
Contributor Author

@willrowe Thanks for the extensive review; I will add the requested tests as soon as possible.

@antoineveldhoven
Copy link
Contributor Author

@willrowe Can you take a look at my revision of this PR?
I;ve added an additional check for negative length parameter in the slice filter as in PHP: array_slice

@willrowe willrowe merged commit ab8791b into twigjs:master Nov 16, 2023
10 checks passed
@JamesB797
Copy link

Nice, thank you for addressing this!

@willrowe
Copy link
Collaborator

@RobLoach can you go ahead and release v1.17.0? Everything in the milestone has been completed, so we might as well release instead of adding more at this point.

@antoineveldhoven antoineveldhoven deleted the slice-filter-variables branch November 17, 2023 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@willrowe willrowe willrowe approved these changes

@RobLoach RobLoach RobLoach approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.17.0
Development

Successfully merging this pull request may close these issues.

The slice filter shorthand doesn't support variables
4 participants
@antoineveldhoven @JamesB797 @willrowe @RobLoach